Repository navigation
fix(eio-client): abort the pending polling requests once the close packet is sent - #5630
Open
mark1russell7 wants to merge 1 commit into
Open
mark1russell7 wants to merge 1 commit into
mark1russell7 wants to merge 1 commit into
Conversation
…cket is sent When the polling transport was closed, it sent a "close" packet, but it did not abort the requests which were still pending, like the long-polling request. If the server (or a proxy) never answered it, the request stayed open after the "close" event, holding a connection (and, in Node.js, preventing the process from exiting). The pending requests are now aborted once the close packet is sent (or cannot be sent). They are not aborted before, so that the server still sees a clean close. Note: with the "xmlhttprequest-ssl" package (Node.js), the requests are not aborted, because aborting a pending request throws an uncaught exception (see mjwwit/node-XMLHttpRequest#37). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The kind of change this PR does introduce
Current behavior
When the polling transport is closed, it sends a
closepacket, but the requests that are still pending (the long-polling GET request, and a POST request in flight) are not aborted. If the server, or a proxy, does not answer them, they stay open after thecloseevent.New behavior
The close packet is still sent first, so that the server sees a clean close. Once the close request is done (or failed), the transport calls a new
abortRequests()method:Fetch: each request (except the close request, which is done at this point) uses the signal of oneAbortControllerof the transport, which is aborted.BaseXHRkeeps the pending requests in a set, and aborts them withRequest#abort(true)(_cleanup(true)callsxhr.abort()).Request#abort()without the argument still only detaches the request, as before (it is used by theunloadhandler).xmlhttprequest-ssl(Node.js):abortRequests()does nothing, becauseabort()on a pending request throws an uncaughtTypeErrorfrom version 2.1.2 (Abort causes TypeError: Cannot read properties of null (reading 'reusedSocket') mjwwit/node-XMLHttpRequest#37).The requests are not aborted before the close request is done on purpose: the Engine.IO server answers the pending GET request when it receives the close packet, and before it answers the close request. If the GET request is aborted at once instead, the server can see it closed before the close packet, and then it closes the session with the reason
transport errorinstead oftransport close.doWrite()gets an optional third argument, called when the request fails, so that the transport knows when the close request is done.Other information (e.g. related issues)
Fixes #5628
The new test in
test/node.js("should abort the long-polling request once the close packet is sent") uses a small HTTP server that answers the handshake and the POST requests, but never answers the long-polling request. When the server receives the long-polling request, it callssocket.close(), and the test expects the server to see the long-polling request closed. It runs withUSE_FETCH=1, and is skipped withxmlhttprequest-ssl(see above).Test results, from one run on a GitHub Actions runner (Linux, Node.js 24.22.2), at the commit 7155a8a of this branch on top of
main(f97d51a). The job followsci-engine.io-client.yml:npm ci, then it compiles engine.io-parser, engine.io-client and engine.io. "Before" is thelib/folder ofmainwith the tests of this branch.xmlhttprequest-ssl)USE_FETCH=1Full suites of the branch:
test:node109 passing, 2 pending.test:node-fetch109 passing, 2 pending.test:node-builtin-ws106 passing, 5 pending.format:checkpasses.The run: https://github.com/mark1russell7/wirelab/actions/runs/43625189318
This PR was prepared with AI assistance (Claude). The tests named above ran before and after the change in that run, with the results above.
🤖 Generated with Claude Code